codex/high roi phase2 - #20
Conversation
BlackishGreen33
commented
Mar 10, 2026
BlackishGreen33
commented
- ⚡ perf(scan-store): reduce writes and track relocation by fingerprint
- ✨ feat(code-scanning): generate guarded sarif in ci pipeline
- 🚀 ci(benchmark): add nightly regression gate for scan metrics
- 📝 docs(steering): sync roi phase2 api and ci rules
There was a problem hiding this comment.
Pull request overview
This PR improves scan-store efficiency and resilience by deduping/relocating vulnerabilities via stableFingerprint, centralizing SARIF generation with size/result guards for CI + export, and adding CI workflows/scripts for Code Scanning SARIF uploads and nightly benchmark regression gating.
Changes:
- Prefer
stableFingerprintfor semantic dedupe and workspace reconcile; add relocation handling that emitsscan_relocatedevents and tracks storage metrics. - Introduce shared SARIF generator (
buildSarifPayloadWithGuards) and use it in/api/exportplus a CI script/workflow to upload SARIF. - Add benchmark regression script + baseline data and a scheduled GitHub Actions workflow.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/server/vulnerability-dedupe.ts | Uses stableFingerprint as the primary semantic dedupe key when present. |
| web/src/server/vulnerability-dedupe.test.ts | Adds coverage ensuring same stableFingerprint dedupes across location changes. |
| web/src/server/sarif-generator.js | New shared SARIF 2.1.0 generator with maxResults/maxBytes guards + warnings. |
| web/src/server/sarif-generator.test.ts | Adds tests for SARIF version, fingerprint preservation, and truncation warnings. |
| web/src/server/routes/vulnerabilities.ts | Includes stableFingerprint in selected fields for stats/dedupe-related behavior. |
| web/src/server/routes/scan.ts | Threads taskId into orchestrators, collects storage metrics, and makes workspace reconcile fingerprint-aware. |
| web/src/server/routes/scan.test.ts | Updates mocks for new orchestrator result fields and adds reconcile relocation regression test. |
| web/src/server/routes/export.ts | Replaces inline SARIF rendering with shared generator + warning header behavior. |
| web/src/server/health-score.ts | Includes stableFingerprint in health-score vulnerability queries. |
| web/src/server/db.ts | Adds vulnerability-only read/write fast paths, relocation upsert flow, storage lock/write telemetry, and new event fields. |
| web/src/server/db.test.ts | Adds relocation + scan_relocated event assertions and validates metrics shape. |
| web/src/server/agents/orchestrator.ts | Returns stableFingerprints + storageMetrics and passes taskId into upsert. |
| web/src/server/agents/agentic-beta/orchestrator.ts | Same as baseline orchestrator: returns fingerprints/metrics and passes taskId. |
| web/src/server/advice-gate.ts | Includes stableFingerprint in advice metric context queries. |
| web/src/common/libs/types.ts | Extends event type union and event payload fields for relocation metadata. |
| web/src/common/components/vulnerability-detail/main.tsx | Adds UI metadata for the new scan_relocated event type. |
| web/scripts/generate-sarif-ci.mjs | CLI script to generate guarded SARIF for Code Scanning uploads. |
| web/scripts/code-scanning-fixture.json | Fixture input for SARIF generation in CI. |
| web/scripts/check-benchmark-regression.mjs | Compares benchmark reports and enforces thresholds after a configurable date. |
| web/package.json | Adds sarif:ci script entry point. |
| web/benchmarks/scan-baseline.json | Adds baseline benchmark numbers used by regression gate. |
| AGENTS.md | Documents new workflows/scripts, SARIF guards, relocation events, and metrics requirements. |
| .kiro/steering/testing-standards.md | Updates testing standards to cover relocation + SARIF guard behavior + regression thresholds. |
| .kiro/steering/tech.md | Documents SARIF generator centralization and regression gating approach. |
| .kiro/steering/structure.md | Updates repository structure docs for new workflows/scripts/baselines. |
| .kiro/steering/product.md | Updates workspace reconcile rule to be fingerprint-aware. |
| .kiro/steering/code-conventions.md | Updates event stream conventions to include scan_relocated + metadata. |
| .kiro/steering/api-standards.md | Updates API rules for fingerprint-aware dedupe/reconcile, SARIF guards, and metrics/log expectations. |
| .github/workflows/code-scanning.yml | Adds workflow to generate + upload SARIF for GitHub Code Scanning. |
| .github/workflows/benchmark-regression.yml | Adds scheduled/manual benchmark regression workflow with enforce-after gating. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (sarif.warnings.length > 0) { | ||
| headers['X-Confession-Sarif-Warning'] = sarif.warnings[0] | ||
| } |
There was a problem hiding this comment.
X-Confession-Sarif-Warning 直接放入中文 warning 內容，Node.js HTTP header value 會因為非 Latin-1 字元而丟 ERR_INVALID_CHAR，導致 SARIF 匯出在發生截斷時直接 500/失敗。建議把 warning 做 ASCII-safe 編碼（例如 encodeURIComponent / base64）或改用可安全承載 UTF-8 的回傳方式（例如 JSON body 欄位或 trailer）。
| if (options.taskId) { | ||
| process.stdout.write( | ||
| `[Confession][StorageWriteMetrics] ${JSON.stringify({ | ||
| taskId: options.taskId, | ||
| ...metrics, | ||
| relocation_count: relocationCount, | ||
| })}\n`, | ||
| ) |
There was a problem hiding this comment.
[Confession][StorageWriteMetrics] 的輸出欄位與 repo 規範不一致：.kiro/steering/api-standards.md 要求至少包含 taskId、vuln_count、write_ops、lock_wait_ms_p95、lock_timeout_count、relocation_count。目前只輸出 taskId + fs_write_ops_per_scan/db_lock_wait_ms_p95/db_lock_timeout_count/relocation_count，也缺 vuln_count。建議補上 vuln_count: normalized.length，並統一欄位命名（或同步更新規範與下游解析）。
| for (const vuln of normalized) { | ||
| const codeHash = createHash('sha256').update(vuln.codeSnippet).digest('hex') | ||
| const exact = await client.vulnerability.findUnique({ | ||
| where: { | ||
| vuln_idempotent: { | ||
| filePath: vuln.filePath, | ||
| line: vuln.line, | ||
| column: vuln.column, | ||
| codeHash, | ||
| type: vuln.type, | ||
| }, | ||
| }, | ||
| select: { id: true }, | ||
| }) |
There was a problem hiding this comment.
在每筆 upsert 前先 findUnique 檢查 exact，之後又呼叫 vulnerability.upsert（內部也會再做一次 findUnique）。在 FileStore 這種 array scan 的實作下，會把查找成本加倍，且大量漏洞時可能趨近 O(n^2)。建議改成一次性建立既有 vuln_idempotent 的 index（Map）或讓 upsert 回傳是否 create/update 來避免額外查找。
| const dedupedFingerprints = Array.from( | ||
| new Set( | ||
| stableFingerprints | ||
| .map((item) => item.trim()) | ||
| .filter((item) => item.length > 0), | ||
| ), |
There was a problem hiding this comment.
buildRelocationQueues 這裡會對 stableFingerprints 做 trim() 後再拿去查詢，但 normalizeVulnerabilityInputsForUpsert 對外部傳入的 stableFingerprint 並沒有先 normalize（只檢查 trim().length > 0，實際存入時保留原字串）。這會造成「寫入的 fingerprint」與「查詢/去重時使用的 fingerprint」可能不一致（例如前後空白/大小寫），進而導致 relocation 命中失敗或重複建立。建議在產生/接收 stableFingerprint 時就統一做 normalize（至少 trim，最好也 toLowerCase）。
